Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc(plc4j): Integrating Kafka and PLC4x using Docker. #892

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

DanubiaM
Copy link

@DanubiaM DanubiaM commented Apr 7, 2023

This pull request adds a section to the documentation in the README file located at https://github.com/apache/plc4x/tree/develop/plc4j/integrations/apache-kafka to describe how to use Kafka and PLC4X with Docker using docker-compose.

Additionally, it includes a docker-compose.yml file that can be used to start four containers: Zookeeper, Kafka, Kafka Connect, and Control Center. The docker-compose.yml file also includes comments that describe the necessary configurations to enable Kafka and PLC4X to communicate correctly.

@DanubiaM DanubiaM marked this pull request as draft April 8, 2023 22:16
@DanubiaM DanubiaM marked this pull request as ready for review April 8, 2023 22:17
@sruehl sruehl requested a review from hutcheb April 11, 2023 08:03
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any major objections to including a docker-compose file based off the Confluent images.

@@ -0,0 +1,101 @@
version: '3'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you've used a mix of confluent versions, latest, 6.0.0 and 6.0.1.

6.0.1, is also far from the latest (7.3.3)

It also might be nicer to have the PLC4X version as an environment variable so it is easier to find when changing it for a release.

I'm guessing you've grabbed a base docker-compose file from Confluent, can you confirm where you got it from so that we can check what license it was released under?



### Start with Docker
If you want to use PLC4x with Kafka on Docker, simply download the docker-compose.yml file, configure the necessary port and IP settings, and start the containers. The available docker-compose.yml file includes four containers: zookeeper, kafka, kafka connect, and control-center. The control-center container provides a web interface to facilitate the configuration of kafka connect. If you don't want to use it, you can remove it from the docker-compose.yml file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor Formatting with PLC4x, should be PLC4X



### Start with Docker
If you want to use PLC4x with Kafka on Docker, simply download the docker-compose.yml file, configure the necessary port and IP settings, and start the containers. The available docker-compose.yml file includes four containers: zookeeper, kafka, kafka connect, and control-center. The control-center container provides a web interface to facilitate the configuration of kafka connect. If you don't want to use it, you can remove it from the docker-compose.yml file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you want to use PLC4x with Kafka on Docker, simply download the docker-compose.yml file, configure the necessary port and IP settings, and start the containers. The available docker-compose.yml file includes four containers: zookeeper, kafka, kafka connect, and control-center. The control-center container provides a web interface to facilitate the configuration of kafka connect. If you don't want to use it, you can remove it from the docker-compose.yml file.
If you want to use PLC4X with Kafka on Docker, simply download the docker-compose.yml file, configure the necessary port and IP settings, and start the containers. The available docker-compose.yml file includes four containers: zookeeper, kafka, kafka connect, and control-center. The control-center container provides a web interface to facilitate the configuration of kafka connect. If you don't want to use it, you can remove it from the docker-compose.yml file.

@chrisdutz
Copy link
Contributor

Yeah ... having a look ... it would be nicer if the PR referenced the Apache Kafka images and not the confluent ones.

@chrisdutz
Copy link
Contributor

Please ignore my last comment ... a quick search on DockerHub showed me that there is no official Apache Kafka docker image, but only those provided by third-parties.

@chrisdutz
Copy link
Contributor

chrisdutz commented Sep 29, 2023

Buuuuut digging even deeper ... I noticed the "cp-enterprise-control-center" image and that sounded non-Apache and indeed ... this is proprietary Confluent stuff that's under the "(Confluent Enterprise License)". So if this is a requirement for this to work, we will not be able to accept this PR.

@DanubiaM
Copy link
Author

DanubiaM commented Oct 5, 2023

Buuuuut digging even deeper ... I noticed the "cp-enterprise-control-center" image and that sounded non-Apache and indeed ... this is proprietary Confluent stuff that's under the "(Confluent Enterprise License)". So if this is a requirement for this to work, we will not be able to accept this PR.

Hi, it makes sense. But I want this to work. If I add a comment with the image name like image: # Please add your Zookeeper image here. , is it okay? I will remove any Confluent references .

If not, I can remove the docker-compose.yml file and rewrite the readme.md to explain how to do this with Docker.

@chrisdutz
Copy link
Contributor

So if you remove the proprietary enterprise manner thing, sure. I did see that there is no official "apache" distribution.

Copy link
Contributor

@sruehl sruehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ottlukas ottlukas added java Pull requests that update Java code integration-kafka-connect https://plc4x.apache.org/users/integrations/apache-kafka.html labels Apr 28, 2024
@chrisdutz
Copy link
Contributor

I still see two references to "confluent" ... these would still need to go.

Copy link
Contributor

@chrisdutz chrisdutz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the remaining references to Confluent

- -c
- |
echo "Launching Kafka Connect worker"
/etc/confluent/docker/run &
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update this to not reference Confluent

CONNECT_CONFIG_STORAGE_REPLICATION_FACTOR: "1"
CONNECT_OFFSET_STORAGE_REPLICATION_FACTOR: "1"
CONNECT_STATUS_STORAGE_REPLICATION_FACTOR: "1"
CONNECT_PLUGIN_PATH: /usr/share/java,/usr/share/confluent-hub-components,/data/connect-jars
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update this to not reference Confluent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration-kafka-connect https://plc4x.apache.org/users/integrations/apache-kafka.html java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants